fix(grammar): stop the Swift try-bang entry shifting past its own type - #1986

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

Good catch, and the fix is right. One piece of paperwork is missing before it can land.

Please add a row to the "Local source patches" table in internal/cbm/vendored/grammars/MANIFEST.md. Any change to vendored sources has to be recorded there, because the vendoring table's claim is that our copy matches the pinned upstream commit — a local patch makes that claim false unless it is written down, and a future re-vendor would silently drop your fix.

You are in good company: three of the four existing rows are the same class of UBSan fix in a vendored scanner.

Suggested row, matching the existing shape:

| swift | `swift/scanner.c`, `OP_SYMBOL_SUPPRESSOR` + `eat_operators` | `1 <<` / `1UL <<` → `1ULL <<` | UBSan: `1 << suppressor` shifts an `int` by up to `TOKEN_COUNT` bits, undefined once the index reaches 31, while the mask it feeds is `uint64_t` |

Worth upstreaming too, if you have the appetite — alex-pinkus/tree-sitter-swift carries the same bug at the pinned commit, and an upstream fix means the next re-vendor drops the local patch instead of re-applying it.

On the change itself, two things I want to name because they are the reason this was easy to review:

You found a bug that the normal test build actively hides. Recovering UBSan prints the diagnostic and continues, so the shift has been executing and being ignored for as long as it has existed. Identifying that the Windows CLANGARM64 leg runs UBSan in trap mode — where the same shift is an illegal instruction rather than a log line — is what turns "a warning nobody reads" into "a crash on one platform".

And you wrote the test's own limitation into the test. "This test cannot go red here… parsing this file at all is the check" is exactly the right comment to leave. A future reader who sees it pass on macOS will not mistake that for evidence. That honesty is worth more than the four-character fix.

Add the manifest row and this is ready.

Manifest row added, and the branch is rebased onto current main.

The row

I used your shape and extended the reason to name both sites, since this PR fixes two:

| swift      | `swift/scanner.c`, `OP_SYMBOL_SUPPRESSOR` + `eat_operators` | `1UL <<` / `1 <<` → `1ULL <<` | UBSan: `1 << suppressor` shifts an `int` by up to `TOKEN_COUNT` bits, undefined once the index reaches 31, while the mask it feeds is `uint64_t`. `1UL << FAKE_TRY_BANG` is the same defect on Windows, where `unsigned long` is 32 bits and `FAKE_TRY_BANG` is 32; the CLANGARM64 leg runs UBSan in trap mode, so there it is an illegal instruction rather than a log line |

The commit touches a second file, and that is deliberate rather than scope creep. MANIFEST.md is itself listed in scripts/vendored-checksums.txt, so writing the row breaks its own recorded checksum unless that line moves with it.

The rebase, and what the conflict was

The branch was 44 commits behind and conflicting. The conflict was one line, and it was that same self-reference: MANIFEST.md's checksum, bumped on main by another patch row and bumped here by mine.

I resolved it by recomputing the hash from the merged file rather than taking either side. Both recorded values described a MANIFEST.md that no longer exists once both rows are present, so either choice would have left the file failing its own check while looking resolved.

bash scripts/security-vendored.sh passes on the rebased branch, and the sanitized build is clean:
test-runner extraction pipeline grammar_regression → 591 passed, no failures and no UBSan runtime reports, built with -fsanitize=address,undefined.

Upstreaming

Yes, and I have not done it yet — I will open it against alex-pinkus/tree-sitter-swift and link it here rather than leave you to guess whether it happened. If it lands, the next re-vendor drops this local patch instead of re-applying it, which is the better end state for a row like this.

On the two things you named

The UBSan point is the one I want to be honest about: I did not identify the trap-mode difference from insight. I hit the recovering-UBSan diagnostic while running an unrelated test, went looking for which leg treats it as fatal, and found the CLANGARM64 one. The finding was real; the route to it was ordinary.

The test comment was deliberate. A test that cannot fail on the machine most people run it on is worth less than it looks, and writing that down felt cheaper than having someone later mistake a green macOS run for coverage.

The MANIFEST row is in, and I have to correct two things I said.

The upstream fix is already upstream — I cannot open it

I said I would open this against alex-pinkus/tree-sitter-swift and link it here. I went to do that and found both changes already there, in commits that landed after our pinned revision:

fb63a7004f07 is byte-identical to 3f831936 here, at the same line. We pin 8abb3e8b3325 (2026-03-20), which predates both. So this is a stale pin, not an unfixed upstream bug, and there is nothing for me to send. That retracts the offer you recorded on #1978 — please do not wait on it.

Both fixes still matter here. main today still carries 1UL << at line 131 and 1 << at line 514.

The row I first wrote would have misled a re-vendor

Your reason for wanting the row is that a re-vendor would otherwise silently drop the fix. That reason is exactly right for the other four rows, where upstream never took the change. It inverts for this one. The table says:

Re-vendoring from upstream must re-apply these.

Anyone following that on the Swift row after a re-vendor past 2026-08-10 would hand-apply a change the newer file already has.

So the row differs from your suggested wording in two ways. The reason column now names both upstream commits, our pin, and the dates, and says to delete this row on a re-vendor rather than re-apply it. The table intro gains one clause allowing that case, because it is a property of the table rather than of one row:

Re-vendoring from upstream must re-apply these, unless the reason column names an upstream commit that already carries the change — then drop the row instead.

The location and patch columns are unchanged from your suggestion.

What is not here

Re-vendoring the Swift grammar is the real end state and it is not in this pull request. TOKEN_COUNT goes 33 to 34 between our pin and upstream main, so the generated parser.c moves with the scanner. That is a bigger change and yours to schedule.

bash scripts/security-vendored.sh passes — MANIFEST.md is itself listed in scripts/vendored-checksums.txt, so its own recorded checksum moved with the edit.

I amended the MANIFEST commit rather than stacking a correction on it, so the branch is still three commits. The two code commits are untouched.

Merged as 620613ed. You did more than I asked for, and the extra part is the valuable bit.

I asked for a MANIFEST.md row and suggested upstreaming the fix if you had the appetite. You went and checked whether it was already upstream — and it was, twice: fb63a7004f07 (2026-04-06, upstream #558) for eat_operators, and 6ab8d1d74ebd (2026-08-10) for the OP_SYMBOL_SUPPRESSOR entry. Our pin 8abb3e8b3325 (2026-03-20) predates both, so this is a backport rather than a local invention. That is a materially different fact about the patch than the one I was working from.

And you encoded the consequence in the manifest rather than only in the row:

Re-vendoring from upstream must re-apply these, unless the reason column names an upstream commit that already carries the change — then drop the row instead.

That turns a static instruction into a self-retiring one. Without it, a re-vendor past 2026-08-10 would dutifully re-apply a patch upstream no longer needs, and the local-patch table would accumulate entries nobody can safely remove because the reason for each has been lost. Naming the upstream commit in the row is what makes the deletion decidable later. That is a better answer than the one I proposed.

The original diagnosis stands too: recovering UBSan prints and continues, which is why 1 << suppressor executed unnoticed for as long as it existed, and the Windows CLANGARM64 leg runs UBSan in trap mode where the same shift is an illegal instruction rather than a log line. Writing that lane-binding into the test's own comment — "this test cannot go red here… parsing this file at all is the check" — is the sort of honesty that stops a future reader mistaking a macOS pass for evidence.

Before merging I confirmed the 12-commit gap to main was inert: no commit had touched swift/scanner.c, MANIFEST.md, vendored-checksums.txt or test_extraction.c since your base, so the green described exactly what landed.

Thank you — that is your second merge today, after #2008.